feat(enrichment): harden ASN/locality enrichment, protobuf structure v2, lookup-table metrics - #126
Draft
randomizedcoder wants to merge 5 commits into
Draft
randomizedcoder wants to merge 5 commits into
randomizedcoder wants to merge 5 commits into
Conversation
Move the ipfeed-collector daemon from the internal repo into this single
module and wire its IP-range feeds into xtcp2's per-socket enrichment.
Phase 0 — move (no behavior change):
- binary -> cmd/ipfeed-collector; guts -> internal/ipfeed/*
- drop the tool's go.mod; merge deps into the root module
- replace the self-contained flake with the repo's Nix machinery
(binaries.nix, cli-help-smoke, oci image, checks/tests)
Phase 1 — pkg/ipasn consumer + asnmap producer:
- internal/ipfeed/asnmap: curated networkOwner/provider -> representative
ASN table; Annotate() sets model.Record.ASN before WriteParquet
- pkg/ipasn: gaissmai/bart LPM trie loaded from the collector artifact,
atomic hot-swap, alloc/lock-free Lookup; table-driven + race + bench tests
Phase 2 — hot-path wiring:
- flat-record proto: new inet_diag_msg_socket_dest_network_owner (1018),
threaded through the s3parquet ParquetRow/schema and ClickHouse DDL
- config proto: enrich_asn_enable / asn_db_path / asn_refresh_interval
- pkg/xtcp: initAsnEnricher loads the artifact and (optionally) refreshes on
an interval; applyEnrichment fills dest_asn (1011) + dest_network_owner
(1018) from the destination IP, no-op when disabled or index nil
Design doc: docs/ipfeed-asn-enrichment.md (lookup-structure pros/cons +
in-proc bart recommendation; representative-ASN caveat; BGP RIB deferred).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The standalone go-sec check (gosec, G104 not excluded) flags unhandled
errors that golangci `//nolint` does not silence. Rather than annotate
them, handle each error for real so `nix build .#checks.<sys>.go-sec`
reports zero issues:
- fetch: a 304 carries no body (RFC 7232) so drop that drain entirely;
route the error-path drains through drainStatusErr, which folds any
drain failure into the returned HTTP-status error.
- summary: Print now returns the tabwriter flush error; both callers in
cmd/ipfeed-collector handle it.
- dockermeta: drainClose logs a drain failure (it only forfeits reuse).
- lldp: a SetDeadline failure now aborts Fetch (the conn is unusable).
- nsdiscover: if the recv timeout cannot be set, degrade to (0,false)
rather than risk blocking the per-namespace reconcile path.
- xtcp: a per-namespace handle close failure is logged.
No behavior change on the success paths; go-sec: Issues 0.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Classify each socket's destination as SELF / LOCAL_SUBNET / REMOTE using per-namespace address/route discovery over raw NETLINK_ROUTE (no vishvananda/netlink dependency), evaluated before the ipfeed IP->ASN lookup so on-host and on-link traffic skips the internet ASN feed. - pkg/xtcpnl: rtnetlink DUMP client + wire-format parsers (RTM_GETLINK/GETADDR/ GETROUTE), family-header deserializers, and DumpRtnetlink transport. Real nlmon-captured 7.1.8 fixtures (testdata/7_1_8) with a deterministic extract generator and real-fixture-driven, table-driven deserialize tests alongside the synthetic positive/negative/boundary/corner suites. - pkg/localnet: pure BuildSnapshot/Classify with an atomic-snapshot contract mirroring pkg/ipasn; table-driven + race + real-fixture end-to-end tests. - pkg/xtcp: per-namespace route-socket lifecycle, background refresh with atomic swap, and the dest-locality hot-path step in applyEnrichment. - record/config/schema plumbing: xtcp_flat_record + xtcp_config protos (+regen), parquet schema, ClickHouse DDL, recordfmt column/humanizer. - nix: reproducible `capture-netlink-fixtures` writeShellApplication (nlmon + tcpdump) wired into the modular nix/ tree. - docs/locality-enrichment.md design doc. The connected-subnet rule is family-agnostic (unicast + no gateway + has Dst): real captures show IPv4 connected subnets are scope-link but IPv6 connected subnets are scope-universe, so a scope-link gate would misclassify every IPv6 on-link subnet as REMOTE. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…v2, lookup-table metrics
Review of the stacked ipfeed-collector/ASN and locality enrichment work found
contained but real holes; this commit closes them and adds the operational
metrics for the IP->ASN lookup table.
Protobuf structure v2 (XtcpFlatRecordSchemaVersion 1 -> 2)
- xtcp_flat_record.proto: documented allocation policy (metadata 1-299,
enrichment 300-399, spare 400-999, payload 1000+ per kernel subsystem, every
tag <= 2047); enrichment block regrouped by subject (socket-side 300s,
dest-side 310s/320s, source-side reserved 350-389).
- Payload field names now mirror the kernel struct member they copy and every
payload field carries a trailing comment naming its kernel source
(include/uapi/linux/{inet_diag,tcp,sock_diag}.h, net/ipv4/inet_diag.c).
Renames: tcp_info_{snd,rcv}_wscale, fastopen_client_fail, rttvar, advmss,
notsent_bytes; sk_mem_info_{rcvbuf,sndbuf}; vegas_info_{rttcnt,minrtt};
inet_diag_{cong,cong_enum,tos,tclass,shutdown,class_id,sockopt,cgroup_id}.
Old numbers/names are reserved; post-6.10 tcp_info members pre-assigned
1266-1276 by comment. tools/proto-field-audit gains a kernel-annotation
guard so a payload field cannot land without its source.
- XtcpConfig renumbered into blocks (gRPC-internal only, nothing persists it).
- Propagated to gen/*, ParquetRow/rowFromProto, recordfmt, the ClickHouse
Kafka table + per-version MVs (_v2 table, explicit column aliases for
_v0/_v1), k8s configMaps, hand-written SQL, and docs
(protobuf-formats, parquet-format, record-versioning, socket-analysis).
build/containers/clickhouse/sql/migrations/v2.sql for existing deployments.
Locality classification and lifecycle (pkg/localnet, pkg/xtcpnl, pkg/xtcp)
- RTN_LOCAL uses the route's real prefix; multipath/RTA_VIA/RTA_NH_ID routes
classify as gateway-reached (Remote, oif 0); Self wins over Subnet
regardless of dump order; only RT_TABLE_MAIN/LOCAL are read; unspecified
destinations -> LOCALITY_UNSPECIFIED.
- Failed namespaces are negative-cached with backoff; loopback-only snapshots
are re-dumped; new-namespace dumps per reconcile are capped;
NLM_F_DUMP_INTR is retried; default locality_refresh_interval 60s.
- DumpRtnetlink loop factored into walkNlMsgs with table + fuzz tests over
real nlmon captures; refreshLocality tests via an injected dumper.
ASN enricher hardening (pkg/ipasn, pkg/xtcp, internal/ipfeed, cmd/*)
- ipasn: only io.EOF ends a read, zero usable prefixes is ErrNoPrefixes, a bad
reload never degrades the table in service, ReloadIfChanged skips an
unchanged artifact by stat.
- initAsnEnricher installs the index even when the first load fails so a
late-arriving artifact is picked up on the refresh tick.
- output.WriteParquet is atomic (.tmp + fsync + rename); fetch bodies are
capped at 256 MiB; -enrichAsn/-asnDbPath/-asnRefreshInterval flags + env.
- Tests for the six previously untested feed parsers, summary, envInt,
s3.parseEndpoint; tabularised the remaining ad-hoc tests.
Lookup-table metrics (operational visibility)
- Daemon: function="loadAsn" gauges prefixes / artifactBytes / loadedAt and
summaries build / error duration, published by one loadAsn helper used by
both the start-up load and the refresh tick; ipasn.Index.Stats() backs it.
- Collector: OTel instruments bridged to Prometheus via
go.opentelemetry.io/otel/exporters/prometheus on a private registry and
served as /metrics on the daemon health server. New instruments:
ipfeed.source.records, ipfeed.artifact.records, ipfeed.artifact.size,
ipfeed.write.duration, ipfeed.upload.duration, ipfeed.cycle.duration.
- goVendorHash updated for the new dependency.
E2E and tooling
- self-test check 5f distinguishes bound vs egress records; check 5g asserts
the ASN, owner, locality AND the loadAsn/prefixes gauge (fixture-derived);
client units Restart=always; TestPrintFlags nil-deref fixed.
Verification: go vet/gofmt clean; go test ./... green (-race on localnet,
xtcpnl, ipasn, xtcp, telemetry, health); golangci-lint only the pre-existing
govet unusedwrite; proto-field-audit clean; nix build .#xtcp2-all;
nix run .#test-microvm-lifecycle-x86_64-interface-naming OVERALL PASS
(XTCP2_SELF_TEST_ASN_PASS dest=8.8.8.8:53 asn=15169 owner=google prefixes=3).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Closes every remaining finding in `golangci-lint-comprehensive` (it now
reports 0 issues) and adds the two linters whose absence let the previous
findings rot. Carries the still-uncommitted enrichment build-tag axis and
the microVM/io_uring fixes from the same working branch.
dupl (2 -> 0) — the nats/nsq Send methods
TODO-SOON.md called this the destination *constructors*; it was not.
newNATSDest and newNSQDest differ substantially. The duplicated region is
the Send bodies, and the file's warning that factoring them out would
"re-link that library into every flavor" was unfounded: those bodies
reference zero nats/nsq symbols, only the natsPublisher / nsqProducer
interfaces the files already define.
New untagged, stdlib-only pkg/xtcp/destinations_publisher.go holds
sendViaPublisher; nats, nsq and valkey each delegate with their own
metric label and a closure over their client. valkey is included even
though it escaped dupl on token count — a helper covering two of three
identically-shaped sinks would look arbitrary to the next reader. Mirrors
the writerDest precedent in destinations_stdout.go. A free function, not
a publisherDest struct, so the existing test doubles that construct
&natsDest{...} directly keep working.
All three flavors' existing Send tests pass unmodified; had any needed
editing the delegation shape would have been wrong. `strings -a` on
xtcp2-min still reports 0 for nats, nsq and valkey.
funlen (1 -> 0) — rowFromProto
The length was not the risk. The function was one keyed ParquetRow
literal of 163 assignments and *nothing tested its output* — the schema
tests only reflect over the struct, and BenchmarkRowFromProto discards
the result. A keyed literal makes an omitted field legal Go, so a bad
split would compile, pass every test, and silently write zeros. (Its doc
comment claimed new proto fields "surface here as a compile error",
which was false for exactly that reason. Corrected.)
So the guard came first: destinations_s3parquet_rowfromproto_test.go
walks ParquetRow by reflection and asserts each column against the
same-named proto field — distinct sentinels, type maxima, type minima,
all-zero, plus a second table for the two enum->int32 columns. It was
proven against the *unsplit* function, then deliberately broken by
deleting the TcpInfoMinRtt assignment (it failed and named the field).
Only then the split into fillInetDiag / fillMemInfo / fillTCPInfo /
fillCCAlgos, along the schema struct's own section boundaries.
misspell (35 -> 0) + prealloc (1 -> 0), and the gate moved earlier
The reported count was wrong: golangci-lint defaults to
max-same-issues: 3 and max-issues-per-linter: 50, so 16 findings were
silently dropped — in files the capped output never named. All three
configs now set both caps to 0; a run whose output is the work queue has
to show all of it.
Swept US, which the repo had already settled four times (most recently
bd3081c, an ancestor of main). The 35 were regressions, not legacy: every
file holding one was added after that sweep. A fifth sweep alone would
have guaranteed a sixth, so misspell is now enabled in Tier 1 with
locale: US and the convention is written into CONTRIBUTING.md. 38 lines
across 26 files, reviewed individually: comments, test `description`
strings and t.Error text only — no identifier, metric label or
user-visible string changed. camelCase tokens are outside what misspell
can see, and proto/headers/sock.c stays verbatim kernel source.
prealloc stays in Tier 2 on purpose: one `continue` anywhere in a file
silences every hint in it, which makes it a poor gate.
deadnix + statix as gating checks (TODO-SOON.md 4c)
Nothing linted the Nix tree, which is why the dead oci-xtcp2.nix
survived for months. nix/checks/{deadnix,statix}.nix follow nix-fmt.nix
and share its vendor/.git/build exclusions.
deadnix: 25 findings, fixed by hand — `deadnix --edit` rewrites the
lambda pattern but not the call sites, so dropping an unused `lib` from a
pattern without `...` turns every `inherit pkgs lib` at a caller into an
eval error. Two removals cascaded (devshell.nix, protos/default.nix).
overlays.nix's overlay argument is now _prev. `nix flake show
--all-systems` is byte-identical before and after except for the two new
check attrs — that diff is what proves no call site was missed.
statix: the 19 W04 manual_inherit_from are fixed. The 15 W20
repeated_keys are disabled in a new repo-root statix.toml with the
reasoning written out: NixOS module attrsets are idiomatically flat
dotted paths grouped by concern, and complying would mean merging 26
systemd.* assignments spread through a 2900-line mkVm.nix into one block,
preventing no defect. A linter-scope decision in config, not a
suppression — there are no `# statix: ignore` comments anywhere.
Both checks were proven to gate: injecting a dead let binding turns
deadnix red naming it; injecting a manual inherit turns statix red.
-race on the io_uring package test (TODO-SOON.md 3a follow-up)
mkPkgTest takes a per-package `race` flag (adds gcc, CGO_ENABLED=1,
passes -race), on for pkg-io-uring only. test-go-race already covers
every package; what was missing was a cheap localised run for the one
where the IORING_SETUP_SINGLE_ISSUER contract lives. The other eleven
stay CGO_ENABLED=0 so the fast path stays fast.
Docs
TODO-SOON.md: 1, 2 and 4c struck as fixed, each carrying what the
original entry got wrong, plus a new "Known recurrence vectors" section
(the issue-cap default, the unpinned golangci-lint, prealloc's coarse
suppression, and `nix flake show` as the eval canary for Nix refactors).
CONTRIBUTING.md gains the US-spelling convention, the Nix-linting
paragraph and the localised race target. tools/quality-report/main.go
moves misspell to Tier 1 in both the tier map and the report template,
and adds the forbidigo entry that was missing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three stacked commits, all new to
main:b8f9b0bfeat(ipfeed): foldipfeed-collectorinto xtcp2 + IP→ASN enrichment (pkg/ipasn,-enrichAsn).db40c79feat(locality): destination locality enrichment via native rtnetlink (pkg/localnet,enrich_socket_dest_locality, egress ifindex/ifname).62a4673feat(enrichment): review-driven hardening of both, a full protobuf structure pass (schema v2), and Prometheus metrics for the lookup tables.Draft while the ClickHouse v2 migration and the schema-version bump get a second pair of eyes.
Protobuf structure v2 —
XtcpFlatRecordSchemaVersion1 → 2xtcp_flat_record.protonow documents the allocation policy (metadata 1–299 · enrichment 300–399 · spare 400–999 · payload 1000+ one block per kernel subsystem · every tag ≤ 2047 stays 2 bytes on the wire). Enrichment block regrouped by subject; source-side range reserved.tools/proto-field-auditgains a guard so a payload field cannot land without one.reserved):tcp_info_{snd,rcv}_wscale,tcp_info_fastopen_client_fail,tcp_info_rttvar,tcp_info_advmss,tcp_info_notsent_bytes,sk_mem_info_{rcvbuf,sndbuf},vegas_info_{rttcnt,minrtt},inet_diag_{cong,cong_enum,tos,tclass,shutdown,class_id,sockopt,cgroup_id}. Post-6.10tcp_infomembers pre-assigned 1266–1276 by comment.XtcpConfigrenumbered into blocks (gRPC-internal; nothing persists the binary form).gen/*,ParquetRow/rowFromProto,recordfmt, ClickHouse Kafka table + per-version MVs (new_v2table, explicit column aliases for_v0/_v1), k8s configMaps, hand-written SQL, docs.Migration for existing ClickHouse deployments:
build/containers/clickhouse/sql/migrations/v2.sql. Parquet consumers should branch onschema_version(seedocs/record-versioning.md,docs/parquet-format.md).Correctness fixes
RTN_LOCALuses the route's real prefix; multipath /RTA_VIA/RTA_NH_IDroutes classify as gateway-reached; Self beats Subnet regardless of dump order; onlyRT_TABLE_MAIN/LOCALread; unspecified dest →LOCALITY_UNSPECIFIED.NLM_F_DUMP_INTRretry, default refresh 60 s.io.EOFends a read; zero usable prefixes isErrNoPrefixes; a bad reload never degrades the table in service; stat-gatedReloadIfChanged; index installed even when the first load fails so a late artifact is picked up on the tick..tmp+ fsync + rename), 256 MiB body cap, six previously untested parsers now covered.Lookup-table metrics (operational visibility)
Daemon, following the existing
function/variable/typepattern underfunction="loadAsn":xtcp_gauges{...variable="prefixes"}xtcp_gauges{...variable="artifactBytes"}xtcp_gauges{...variable="loadedAt"}xtcp_histograms{...variable="build"}xtcp_histograms{...variable="error"}Collector (
-daemon -http-addr): OTel instruments bridged to Prometheus on/metricsnext to/healthzand/readyz. New:ipfeed_source_records{source,provider},ipfeed_artifact_records,ipfeed_artifact_size_bytes,ipfeed_write_duration_seconds,ipfeed_upload_duration_seconds,ipfeed_cycle_duration_seconds{outcome}. Operator table incmd/ipfeed-collector/README.md.Test plan
go vet ./...,gofmt -l .clean;go test ./...green (-raceon localnet, xtcpnl, ipasn, xtcp, telemetry, health)golangci-lint run— only the pre-existing govetunusedwriteinpoller_pure_test.gogo run ./tools/proto-field-auditclean incl. the new kernel-annotation check;nix run .#regen-protosleaves the tree cleannix build .#xtcp2-all(newgoVendorHash)nix run .#test-microvm-lifecycle-x86_64-interface-naming→XTCP2_SELF_TEST_OVERALL_PASS, all 20 checks, includingXTCP2_SELF_TEST_ASN_PASS (dest=8.8.8.8:53 asn=15169 owner=google prefixes=3)andXTCP2_SELF_TEST_IFNAME_PASSclickhouse local(Kafka engine stood in by a MergeTree table with the same columns), two scenarios:initdb.dinstall: epoch 0/1/2 rows route to_v0/_v1/_v2, Merge view shows all three,_v2has 161 columns;main's DDL with an epoch-1 row already stored, thenmigrations/v2.sql: the stored row survives with its old column names, new epoch 0/1/2 rows route correctly,SELECT schema_version, count() FROM xtcp.xtcp_flat_records GROUP BY 1shows all epochs.tcp-stress/minimallifecycle flavors on a quiet host (the minimal flavor's 180 s deadline is starved on a loaded box)Pre-existing reds on
mainindependent of this PR: go-sec G104 ×4, s3parquet schema-drift check, xsync race flake.🤖 Generated with Claude Code